I wonder if there is a way to have the tests generate these HTML files with Sphinx, since I imagine this kind of stuff will vary across sphinx version? It would be awesome to have a matrix of support along the axises where this kind of stuff changes.

@ericholscher so, right now what we check is that every section is interpreted correctly by the embed app, that means having the original html file and the html of each section. We can't generate the html of each section automatically since that's what we are testing. For testing mkdocs across different versions/themes for the search app I just created another version in the tests (e.g. bibtet-material, bibtext-basic, etc), so we can add tests for more versions and themes, but we still need to "write" the expected html of the section "by hand" (this is actually just a copy/paste/replace thing)

btw, what it takes most of the time is just finding a live example with what to test :)

@stsewd not sure I follow. Presumably this HTML is generated from Sphinx, so can't we generate it?

@ericholscher I get this html from sphinx to test it. But we can't automate this, since we need to test that for given a html page (input) we get the expected html sections (output).

So, currently we have a html page (input) and multiple pages representing each section (output). We test that the sections match what we expect input->sections = section from output.
So, to test several versions/themes we can just copy the generated html from that version and write more tests with what we expect as sections.

If you are talking about automating only the input page, we can, but that will only work with one version (the current we have installed), and we also need to provide a full project that generates the html we want to test, right now we can get the html from only the pages we want to test from any project. So, it is more easy to just copy the generated html from a real project.

I wonder if there is a way to have the tests generate these HTML files with Sphinx, since I imagine this kind of stuff will vary across sphinx version?

@ericholscher @stsewd I'm doing this in sphinx-notfound-page (https://github.com/readthedocs/sphinx-notfound-page/blob/master/tests/test_urls.py#L46) and sphinx-hoverxref (https://github.com/readthedocs/sphinx-hoverxref/blob/master/tests/test_htmltag.py#L34). There are different "project examples" that we use multiple times to test different things. The test itself builds the Sphinx project and generates the HTML. Then the test uses that HTML generated as input and check it contains what we are looking for.

For the Embed API, we could do the same and mock the storage to return the "just generated HTML" so the API uses it as a source to parse it and return the correct chunk of code (that we should have hard-coded in our tests)

For sphinx we don't use the html directly, but the metadata from the jsonf files. And for mkdocs we would have to do a system call since it doesn't have that kind of integration as sphinx. Feels complicated just to generate the input html instead of just copying it (and seeing the differences from the expected html would be hard since we would have rst -> html sections instead of html -> html sections. Also, to test several versions we would need to run these tests in another tox env to be able to change the sphinx version. For the extension makes sense, but for the backend code I'm not sure. We should make the parser more generic and add more tests with different html from several themes/version instead of depending too much on the sphinx specific structure.

In fact, due the reasons you are mentioning here, I'm thinking that it may be good to have the embed code in a different repository then. In particular to be able to run tests across different Sphinx versions --if that's not possible to do it with the current tox setup.

Harcoding the input is not bad, but definitely we are not testing this code completely. Different Sphinx versions could generate different HTML/jsonf (even with the same version using HTML4 or HTML5). We don't need to "generate the expected output", that's fine to be hardcoded for each different version if they are not equal for some reason.

I think it's better to have Sphinx tests' using the correct way and MkDocs tests' hacked, than both hacked.

Different Sphinx versions could generate different HTML/jsonf (even with the same version using HTML4 or HTML5).

We can test that too, we just need to copy the generated output. I don't think we should move the repo just to change how the testing is done. Don't think using the sphinx helper improves anything, is more explicit having html as input rather than rst.

I think what we want is:

• Hard-coded responses as the expected API response
• Dynamic generation of the HTML input via a matrix of Sphinx versions

The primary thing I'm looking for here is we want to ensure new versions of Sphinx don't break our API responses, and code changes don't break old versions of Sphinx. If we are hard coding both the input & output, we aren't actually testing Sphinx generation across versions and extensions, which is a major issue we're trying to address with this these tests. We want to test specific rst input across versions of sphinx generates HTML we properly parse.

There is also a larger question here which is: when the HTML changes across versions, do we want to pass that HTML change along to embed API clients? I can see both sides to this, and it should be addressed in our design doc for v3 👍

Okay, so I added tests generated from the sphinx output:

• Had to create another tox config file, since it doesn't support parametrization of the other environments Allow parametrization of any testenv tox-dev/tox#189
• I'm using our own extension to generate the fjson files
• Tests are being executed in another job
• Tests are being run with our current installed sphinx version too (this is when running the normal tests)
• Looks nothing changed from sphinx 1.8 to 3.5. With the basic elements at least.
• Our CI time increased from ~7 to ~13 min

Things I noticed:

• tests are slow (we can reduce the version matrix, but then if doesn't make much sense to run so tests against different sphinx versions)
• Nothing has changed between versions (at least with the elements present in the rst sources). So do we really need to test against all versions?
• HTML from tests cant be formatted since we expect the same whitespace/order from the html generated from the source files. Making it a little hard to diff, not that we would be expending too much time here I guess, but still hard when we would want to add new things.
• Since several rst pages are from the same project, changes in one file may change others.
• Some elements we want to test (like tabs, bibtext, etc) are present in extensions, so we need to install and setup those in order to test them. We could exclude these tests from the "normal" tests completely and create another requirements file with all these deps, but we still need to setup these extensions.

So, I'm a little biased here bc I like the pure html tests more... but I don't see benefits from using source files here as the html structure hasn't changed for all those versions. What I can see changing more is the output from multiple versions of extensions/themes. And even if the html changed, it could be maybe white space changes. So I think we should focus more on the different structures that are generated from different themes/extensions and make our parsing code work on those structures in a general way (like how sections or definition lists are organized) and of course this is more easy to test with pure html as input instead of having a large matrix of themes and extensions. Plus, no need for another setup to run these tests.

I left the old tests around since they are already written and cover more things, we can delete them if we move fully to the other type of tests.

Think I'm actually testing with the same sphinx version, I'll try something tomorrow

Edit: nope, all good.

@stsewd I don't quite understand why we aren't using tox to just run the full test suite against each version? That seems like the most explicit way to do this.

Oh, because it's diff than the RTD tests. It feels like we're reaching a point where we might be better off having the embed code in a separate repo, if it's causing this much headache.

I think we came down with:

• We should define a contract for standard HTML elements, with tests being closer to unit tests and less about full-page tests
• Then run those tests against the sphinx matrix for now.
• If we hit speed or other issue, we can look at outputting the HTML from the matrix and caching it somehow

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

I think this PR can be closed now we implemented EmbedAPIv3.